Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use moment-range overlaps method #155

Merged
merged 2 commits into from
Apr 26, 2018

Conversation

chadnickbok
Copy link
Contributor

@chadnickbok chadnickbok commented Sep 6, 2016

The moment-range overlaps method doesn't consider ranges which 'touch' on equal start/end moments to be overlapping. This breaks highlighting and selection display across month boundaries.

Instead, explicitly test if the first range contains the start and end days of the second range after checking for overlap.

The moment-range `overlaps` method doesn't consider ranges which 'touch' on equal start/end moments to be overlapping. This breaks highlighting and selection display across month boundaries.

Instead, explicitly test if the first range contains the start and end days of the second range, as well as checking for overlaps.

Oops again
@AlanFoster
Copy link
Member

AlanFoster commented Sep 7, 2016

@chadnickbok Thanks for the PR: Is it possible to add a test for this scenario? 👍

@chadnickbok
Copy link
Contributor Author

chadnickbok commented Sep 8, 2016

I added a simple test that fails in current master, but passes in my fork.

Let me know if there's anything more I can do to help get this merged :)

@AlanFoster
Copy link
Member

Thanks for the PR! 👍

@AlanFoster AlanFoster merged commit d657e4d into onefinestay:master Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants